Extend registry checks to the development registry#9233
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fc23d6a-ef10-4f60-b369-2770f0af80d6
|
Azure Pipelines: 22 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: 22 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Extends extension registry approval checks and ownership rules to the development registry.
Changes:
- Detects and validates changes to either registry, including renames.
- Supports registry-only updates affecting one or both registries.
- Adds equivalent CODEOWNERS coverage and tests.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ext-registry-check.yml |
Detects changes to both registries. |
.github/scripts/src/ext-registry-check.js |
Applies policy checks independently per registry. |
.github/scripts/test/ext-registry-check.test.js |
Tests development and dual-registry updates. |
.github/CODEOWNERS |
Assigns development registry owners. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Medium
richardpark-msft
left a comment
richardpark-msft
left a comment
There was a problem hiding this comment.
Small stuff - generally, I dislike default values as they usually just lead to weird bugs. There's no compat requirements for that function, so change the signature at will :)
richardpark-msft
left a comment
richardpark-msft
left a comment
There was a problem hiding this comment.
Sorry, meant to use request changes earlier.
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
The per-registry loop and dual-path detection look correct, and the tests cover the dev-only and both-registries cases. One follow-on tied to the existing request to drop the registryPath defaults: see the inline note on DEFAULT_REGISTRY_JSON_PATH.
Remove implicit registry path and base ref defaults, and include the supported registry paths in validation errors.
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Confirmed the removed defaults are safe: run() passes registryPath and registryBaseRef explicitly at every call site, and falls back to base sha then 'main' before invoking the check. My earlier note about the dangling DEFAULT_REGISTRY_JSON_PATH constant is resolved.
|
/check-enforcer evaluate |
richardpark-msft
left a comment
richardpark-msft
left a comment
There was a problem hiding this comment.
Thanks for addressing the feedback. Now...you just have copilot to deal with.
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
The new commit resolves the two open threads on the last revision. diffChangedFiles now emits separate, correctly-directed messages for non-registry edits versus registry renames, and the added test drives per-path fixtures with one policy-invalid registry so independent per-registry evaluation is actually exercised. Re-approving on the current HEAD.
Closes #9234
This PR extends the existing extension registry approval policy to
cli/azd/extensions/registry.dev.json, applying the same validation and CODEOWNERS rules currently used for the production registry.Changes
ext-registry-checkwhen either production or development registry changes, including rename scenarios.registry.dev.jsonthe same CODEOWNERS asregistry.json.Testing
npm test: 54 tests passed, with 5 opt-in live tests skipped.RUN_LIVE_TESTS=1 npm test: all 59 tests passed, including the existing real GitHub PR scenarios.registry.dev.jsondisplay-name update passed.registry.dev.jsonmetadata change.